Add support for uploading data exports to S3

[tudddorrr]

No description provided.

[claude]

PR Review: Add support for uploading data exports to S3

Overall this is clean, well-structured work. The fallback to email attachments when S3 is not configured is a good design choice for a self-hostable product. A few things worth looking at:

---

Major

dataExport.game relation not previously required by the mailer

In dataExportMailer.ts, the S3 key construction accesses dataExport.game.id. This is a new relation dependency that did not exist before. If game is not populated on the DataExport instance passed to send(), the key becomes data-exports/game-id=undefined/export.zip — a silent data integrity bug that will not throw but will make exports hard to find/manage in S3.

The mailer already relies on dataExport.createdByUser.email, so relations are clearly expected to be loaded by the caller. But given this is a background job (not an HTTP request where the Identity Map helps automatically), it is worth explicitly documenting or asserting that game must be populated — or loading it inside send() if it is not guaranteed.

The S3 test mocks the upload so it will not catch this at test time either.

---

Minor

Missing test: S3 upload failure path in DataExportMailer

The existing tests cover email send failure, and the new tests cover the happy S3 path, but there is no test for when uploadToS3 throws (e.g. auth error, network failure). The try/finally ensures the file is still deleted, which is correct behaviour — but a test would confirm that:

1. The file is cleaned up via fsp.unlink
2. The error propagates (so the caller can mark the export as failed)

s3mini is pre-1.0

s3mini@^0.9.2 is a pre-1.0 package, meaning the ^ range allows breaking changes in minor versions (per semver pre-1.0 conventions). Consider pinning to an exact version until it stabilises, or confirming the library's own stability guarantees.

---

No issues found

• Code quality: The separation between isS3Configured() and uploadToS3() is clean. The try/finally for file cleanup is strictly better than the previous code (which did not guarantee deletion if email queuing threw).
• Security: Presigned URL expiry (7 days) is consistent between the code comment, the constant, and the email copy.
• Test coverage: s3Client.test.ts is thorough — covers all isS3Configured branches and the main uploadToS3 paths.

[codecov]

Codecov Report

❌ Patch coverage is 96.55172% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 97.40%. Comparing base (d162d20) to head (9cbbd78).
⚠️ Report is 5 commits behind head on develop.

Files with missing lines	Patch %	Lines
src/lib/storage/s3Client.ts	93.75%	0 Missing and 1 partial ⚠️

Additional details and impacted files

@@           Coverage Diff            @@
##           develop     #913   +/-   ##
========================================
  Coverage    97.39%   97.40%           
========================================
  Files          405      406    +1     
  Lines         6604     6629   +25     
  Branches       871      878    +7     
========================================
+ Hits          6432     6457   +25     
  Misses          88       88           
  Partials        84       84           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.