3 Commits
Author SHA1 Message Date
bermudalambandClaude Opus 5 5a9022d8d1 test(integration): mock the mailer everywhere an upload link is issued (#260)
intake.integration.test.ts, intakeCeiling.integration.test.ts and uploadLinks.integration.test.ts all create upload links, and since #260 that now sends real mail. env.setup.ts never clears SMTP_USER, SMTP_PASSWORD or MAIL_ALLOWLIST, so with those inherited from a developer's shell these three files opened live TLS connections to smtp.gmail.com:465 and, with no allowlist set, actually delivered to sarah@example.com. This is the exact hazard the "Do not add one back" comment in tests/unit/mailOutcome.test.ts already warns about, reintroduced at the integration layer.

All three now mock ../../src/mailer the same way accountDetails.integration.test.ts, favorites.integration.test.ts and resendVerification.integration.test.ts already do. uploadLinks.integration.test.ts is the one place that needs to see specific MailOutcome values come back through the route, so its two outcome tests were restructured to drive the mock's return value directly (sentMail.mockResolvedValueOnce(...)) instead of threading SMTP_USER/MAIL_ALLOWLIST through the real sendMail. That is a cleaner test anyway: it isolates the route's job (reporting whatever outcome sendMail returns) from sendMail's own skip logic, which is already covered hermetically by mailOutcome.test.ts and mailAllowlist.test.ts.

Also addresses the related minor finding that nothing asserted the mail actually carried the working link: required: ['submitUrl'] on the template only guards that the placeholder is present in the body, not that the route supplied a correct value for it. A new test in uploadLinks.integration.test.ts inspects the mock's captured call and asserts the html contains the created token's /submit/ URL, and covers the three submissionsAllowed phrasings (a numeric cap, a cap of exactly one, and uncapped).

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
2026-09-03 18:30:35 -05:00
bermudalambandClaude Opus 5 b18b3e3a3d feat(intake): require an address for an upload link and send the link to it (#260)
Creating a link now requires a valid email address and mails the link to it, which is the whole point: getting a link to a contributor was previously a copy-and-paste into whatever the admin happened to use.

The send is awaited and its outcome reported, unlike every other sender in this codebase, which fires and forgets because nobody is waiting on the answer. Here somebody is. The admin is looking at the screen, and whether they now have to send the link by hand is exactly the thing they need to know — and QA blocks delivery to any address outside MAIL_ALLOWLIST by design, so a link that was never emailed would otherwise look precisely like one that was.

A send that could not happen does not roll the link back. The token is displayed exactly once, so a rollback would leave the admin retrying and holding a different link, discarding work that had succeeded. They end up with a usable link and an honest statement about delivery instead.

One inaccuracy left deliberately: an SMTP rejection is reported as skipped-unconfigured rather than a fourth outcome of its own. The distinction is real but nothing consumes it, and the admin's next action is identical either way.

Also updates the other integration tests that created a link with only a label, since an address is now required, and adds the uploadLink template key that GET /api/admin/email-templates was missing from its list — an omission left by the template's addition in the prior commit on this branch.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
2026-09-03 18:00:15 -05:00
bermudalambandClaude Opus 5 92c04847d6 feat(intake): refuse submissions past the daily ceiling, and reset it from the admin (#227)
SonarQube Analysis / sonarqube (pull_request) Successful in 26m27s
Linting / lint (pull_request) Successful in 2m5s
The check is ordered ahead of uploadImages, for the same reason requireUsableLink is: a refused submission must write zero bytes. Ordering it after would accept the upload, store the files and then throw them away, which is the expensive half of the work the ceiling exists to prevent. A test asserts nothing is stored.

503, not 403. The sender has done nothing wrong, their link is fine, and the condition clears by itself as the window rolls — so the link stays usable and works again the moment there is room.

The ceiling never touches the admin upload path, which has its own test. Intake being throttled is an inconvenience; the shop being unable to add its own stock is an outage.

reset-ceiling is declared above /:id/revoke because Express matches in order and would otherwise read it as an id and try to revoke a link named "reset-ceiling". That has its own test too.

The per-link alert is a named function rather than the inline IIFE the plan wrote. routesAreWrapped.test.ts flags any async inside a route registration not directly wrapped in asyncRoute, and it cannot tell an inner IIFE from an unwrapped handler — nor should it have to. The guard caught this, and the extraction reads better than what it rejected.

Backend now 373 unit and 337 integration, all passing.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
2026-09-01 16:29:00 -05:00