diff --git a/docs/superpowers/specs/2026-09-03-upload-link-email-design.md b/docs/superpowers/specs/2026-09-03-upload-link-email-design.md new file mode 100644 index 0000000..51e9253 --- /dev/null +++ b/docs/superpowers/specs/2026-09-03-upload-link-email-design.md @@ -0,0 +1,91 @@ +# Emailing an upload link to the person who will use it + +**Issue:** #260. Builds on #222 (upload links) and #224 (the intake notification, which is the closest existing mail). + +Today an admin creates an upload link, the token is shown once in the console, and getting it to a contributor is a copy-and-paste into whatever the admin happens to use. This makes the address part of creating the link, and sends the link to it. + +## Decisions, and what each one rests on + +**The mail carries the working link.** The issue framed this as a loosening comparable to a password reset, and that framing overstates it. A reset token takes over an account; an upload token grants exactly one capability — submit photos into a queue where a person must approve them before anything is published. It reads nothing, it is revocable, `max_submissions` caps it, and #227 caps the whole intake surface regardless. The worst outcome of a leaked upload link is junk in the review queue, which is bounded and reversible. That is a reasonable thing to put in an inbox; a reset link is a much larger bet and this project already makes that one. + +**The address is required for new links, and the column is nullable.** Those are not in tension: links already exist in QA and a migration cannot invent addresses for them, so they are grandfathered at null, and the requirement lives in the route where new links are made. Making the column `NOT NULL` would mean either inventing data or a backfill that lies. + +**The address lives on the link, not on a contributor.** A link already carries a label naming who it is for. A contributor entity would be the better model only if the same people submit repeatedly through different links, which nothing yet suggests, and it is materially more work. + +**A failed send does not lose the link.** The token is displayed exactly once, so rolling the creation back on a send failure would leave the admin retrying and getting a different link — harmless but confusing, and it throws away work that succeeded. Instead the link is created, the send is attempted, and the response says which happened. + +That last point is not defensive programming for its own sake. **QA sets `MAIL_ALLOWLIST` and silently skips any address outside it**, logging `[mail-blocked]` and returning as though it sent. Without an explicit outcome in the response, testing this in QA against a contributor's real address looks exactly like success. The issue itself warns this would otherwise waste an afternoon. + +## Architecture + +``` +Admin | Upload Links + label + email + cap + │ + ▼ +POST /api/admin/upload-links + ├─ isValidEmail(email) 400 naming the field + ├─ INSERT upload_links contact_email alongside label and cap + ├─ renderTemplate('uploadLink') submitUrl required, as verification requires verifyUrl + └─ sendMail(...) → outcome 'sent' | 'skipped-unconfigured' | 'skipped-blocked' + │ + ▼ +201 { ...link, token, url, mail: { sent: boolean, outcome: MailOutcome } } +``` + +### `sendMail` gains a return value + +`sendMail` currently returns `Promise` and returns early in two cases — SMTP unconfigured, and the recipient not on `MAIL_ALLOWLIST` — both indistinguishable from success at the call site. It will return a small result instead: + +```ts +export type MailOutcome = 'sent' | 'skipped-unconfigured' | 'skipped-blocked'; +``` + +**No existing caller changes.** There are seven, and every one ignores the return value; ignoring a returned value is legal, so this is additive rather than a breaking change. The alternative — re-deriving "would this address be blocked?" in the route — would duplicate `isAllowedRecipient` and the SMTP check at a second site, which is exactly the drift the guard-in-one-place comment in `mailer.ts` exists to prevent. + +### Data + +`upload_links.contact_email TEXT` — nullable, no default. Null means a link made before this existed, or by a caller that predates the requirement. Nothing reads it except the admin list and the mail at creation. + +### The template + +A new `uploadLink` key in `TEMPLATES`, editable in the admin like the others. + +- `required: ['submitUrl']` — the same guard that stops a verification mail shipping without its link. A link email with no link is the one failure worth making impossible. +- `available: ['submitUrl', 'label', 'submissionsAllowed']` +- `submissionsAllowed` renders as a number for a capped link and as words for an uncapped one, so an unlimited link reads as a sentence rather than as a missing value. Pinned exactly, so it is not a coin-flip at implementation time: a capped link renders `25 items`, or `1 item` at a cap of one; an uncapped link renders `as many items as you like`. The default body uses it as "You can send {{submissionsAllowed}}." + +### The admin screen + +Email becomes a required field in the create form, beside the label. The links table gains the address, so it is visible who a link was sent to. When the response reports the mail was not sent, the admin is told plainly and the link is still shown to copy. + +## Failure handling + +| What happens | Result | +|---|---| +| Address missing or malformed | 400 naming the field. No link created. | +| SMTP not configured (local development) | Link created. `mail.sent` false, `mail.outcome` `'skipped-unconfigured'`. | +| Address not on `MAIL_ALLOWLIST` (QA) | Link created. `mail.sent` false, `mail.outcome` `'skipped-blocked'`. | +| SMTP rejects the message | Link created. `mail.sent` false. The admin copies the link by hand. | +| Everything works | Link created, `mail.sent` true, `mail.outcome` `'sent'`. | + +`mail.outcome` is always present and always one of the three values, including on success — a field that appears only on failure is one every consumer has to remember to check for. + +A send failure never rolls back the link, and never turns into a 500. The admin always ends up holding a usable link and an honest statement of whether it was delivered. + +## Testing + +- **Integration:** the address is required; a malformed one is refused naming the field; a valid one is stored; the mail is attempted with the rendered template; a send failure still returns 201 with a usable token and `mail.sent` false. Existing links with a null address still list and still work. +- **Unit:** the `uploadLink` template refuses a body with no `submitUrl`, through the existing `missingPlaceholders` guard. +- **E2E:** the create form refuses to submit without an address, and a created link shows its address in the table. +- **Existing callers:** the e2e fixtures and `admin-upload-links.spec.ts` create links with only a label and must be updated. That is the cost of the requirement, and the compiler and the suite find every one. + +## Out of scope + +**Re-sending a lost link.** Only the digest is stored, so the original token cannot be recovered. "Re-send" would mean minting a new link and revoking the old one — a different feature with its own decisions about what the recipient is told. + +**Notifying the address on revoke.** Not asked for, and it is a separate judgement about whether a contributor should learn their link was withdrawn. + +**A contributor entity.** See the decisions above. + +**Any change to the seven existing `sendMail` call sites.** They keep ignoring the outcome. Making each of them report delivery is a larger piece of work with no demand behind it.