2026-09-03 18:50:37 -05:00
1 changed files with 91 additions and 0 deletions
@@ -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<void>` 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.