feat: add GET /api/streams/export.csv endpoint - #661
Conversation
- Implement CSV export with streaming (res.write/res.end) - Reuse listStreamsQuerySchema validation and filter logic - Extract shared applyStreamFilters() and csvEscape() helpers - Refactor list, recipients, and senders routes to use shared filter - Set Content-Type text/csv and Content-Disposition attachment headers - CSV columns: id, sender, recipient, asset, totalAmount, vestedAmount, status, startAt, durationSeconds, createdAt - Add 6 supertest-based tests (happy path, filtering, empty, format, large dataset, validation errors) - Fix pre-existing duplicate declarations in validateEnv.ts and duplicate code block in recipients route
|
@elcabasa is attempting to deploy a commit to the ritik4ever's projects Team on Vercel. A member of the Team first needs to authorize it. |
|
@elcabasa Great news! 🎉 Based on an automated assessment of this PR, the linked Wave issue(s) no longer count against your application limits. You can now already apply to more issues while waiting for a review of this PR. Keep up the great work! 🚀 |
📝 WalkthroughWalkthroughThe PR centralizes stream filtering across listing routes, streams CSV exports incrementally with escaped fields, adds comprehensive export tests, and simplifies environment validation control flow. ChangesStream export and filtering
Environment validation cleanup
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant Client
participant ExportRoute
participant Response
Client->>ExportRoute: GET /api/streams/export.csv with query filters
ExportRoute->>ExportRoute: filter streams and escape CSV fields
ExportRoute->>Response: write CSV header and rows
ExportRoute->>Response: end response
Response-->>Client: CSV download
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
backend/src/index.test.ts (2)
792-826: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUnused streaming-verification helper; the one test that should use it doesn't.
invokeExportCsvRoutecaptureswrite()calls incrementally, but no test calls it — the "streams large datasets without buffering" test usesrequest(app)and only checks the final concatenated body, so it never actually proves incremental/non-buffered writes despite its name.
backend/src/index.test.ts#L792-L826: either remove this helper if unneeded, or wire it into the large-dataset test below.backend/src/index.test.ts#L883-L915: useinvokeExportCsvRoutehere and assertchunks.length > 1(or similar) so the test actually verifies incremental streaming rather than only final content.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@backend/src/index.test.ts` around lines 792 - 826, The unused invokeExportCsvRoute helper in backend/src/index.test.ts:792-826 must be wired into the large-dataset streaming test at backend/src/index.test.ts:883-915, replacing request(app) there and exposing or returning write-call information so the test asserts multiple chunks were written; no direct removal is needed at the helper site because it becomes used by the sibling test.
869-881: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winTest doesn't exercise
csvEscape's special-character handling.Header/column-count checks are good, but no test data contains commas, quotes, or newlines, so the quoting/escaping branch of
csvEscape(backend/src/index.tsLines 230-236) is never exercised.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@backend/src/index.test.ts` around lines 869 - 881, The CSV export test around “produces valid CSV parseable by standard parser” must include stream data containing commas, quotes, and newlines, then parse the response with a standard CSV parser and assert the original field values are preserved. Ensure the fixture reaches csvEscape and validates escaped fields rather than only splitting lines and columns manually.backend/src/index.ts (1)
11-11: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueRemove the unused
Readableimport.
Readableis only referenced in the import statement, so removing it keeps the API file cleaner.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@backend/src/index.ts` at line 11, Remove the unused Readable import from the module imports in index.ts, leaving the remaining imports and API implementation unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@backend/src/index.ts`:
- Around line 711-717: Update the CSV export flow around applyStreamFilters to
call listStreams with query.include_archived, query.sort ?? "createdAt", and
query.order ?? "desc", matching the list, recipient, and sender routes. Preserve
the existing progress mapping and filtering behavior while ensuring export
respects validated sort/order parameters.
---
Nitpick comments:
In `@backend/src/index.test.ts`:
- Around line 792-826: The unused invokeExportCsvRoute helper in
backend/src/index.test.ts:792-826 must be wired into the large-dataset streaming
test at backend/src/index.test.ts:883-915, replacing request(app) there and
exposing or returning write-call information so the test asserts multiple chunks
were written; no direct removal is needed at the helper site because it becomes
used by the sibling test.
- Around line 869-881: The CSV export test around “produces valid CSV parseable
by standard parser” must include stream data containing commas, quotes, and
newlines, then parse the response with a standard CSV parser and assert the
original field values are preserved. Ensure the fixture reaches csvEscape and
validates escaped fields rather than only splitting lines and columns manually.
In `@backend/src/index.ts`:
- Line 11: Remove the unused Readable import from the module imports in
index.ts, leaving the remaining imports and API implementation unchanged.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 31393b17-332c-4d2b-923c-21ecf3c522c4
📒 Files selected for processing (3)
backend/src/config/validateEnv.tsbackend/src/index.test.tsbackend/src/index.ts
| const data = applyStreamFilters( | ||
| listStreams(query.include_archived).map((stream) => ({ | ||
| ...stream, | ||
| progress: calculateProgress(stream, now), | ||
| })), | ||
| query, | ||
| ); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
CSV export ignores sort/order query params.
The list route (Line 605) calls listStreams(query.include_archived, query.sort ?? "createdAt", query.order ?? "desc"), and the recipient/sender routes (Lines 1030, 1081) do the same. The export route calls listStreams(query.include_archived) with no sort/order args, so it always returns createdAt-desc ordering even when a caller supplies sort/order (which the shared schema still validates). This breaks parity between list and export for the same query params.
🐛 Proposed fix
const data = applyStreamFilters(
- listStreams(query.include_archived).map((stream) => ({
+ listStreams(query.include_archived, query.sort ?? "createdAt", query.order ?? "desc").map((stream) => ({
...stream,
progress: calculateProgress(stream, now),
})),
query,
);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const data = applyStreamFilters( | |
| listStreams(query.include_archived).map((stream) => ({ | |
| ...stream, | |
| progress: calculateProgress(stream, now), | |
| })), | |
| query, | |
| ); | |
| const data = applyStreamFilters( | |
| listStreams(query.include_archived, query.sort ?? "createdAt", query.order ?? "desc").map((stream) => ({ | |
| ...stream, | |
| progress: calculateProgress(stream, now), | |
| })), | |
| query, | |
| ); |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@backend/src/index.ts` around lines 711 - 717, Update the CSV export flow
around applyStreamFilters to call listStreams with query.include_archived,
query.sort ?? "createdAt", and query.order ?? "desc", matching the list,
recipient, and sender routes. Preserve the existing progress mapping and
filtering behavior while ensuring export respects validated sort/order
parameters.
|
Hi @elcabasa, This PR could not be merged because it has merge conflicts with the target branch. Please resolve the merge conflicts, push the updated changes, and the PR can be reviewed and merged. Thank you! |
2 participants
Summary
closes #601
res.write/res.end) atGET /api/streams/export.csvlistStreamsQuerySchemavalidation and filter logicapplyStreamFilters()andcsvEscape()helpersGET /api/streams,/api/recipients/:accountId/streams, and/api/senders/:accountId/streamsto use shared filter functionCSV Columns
id, sender, recipient, asset, totalAmount, vestedAmount, status, startAt, durationSeconds, createdAtResponse Headers
Content-Type: text/csvContent-Disposition: attachment; filename="streams.csv"Tests
6 new supertest-based tests:
Pre-existing Fixes
validateEnv/isProductiondeclarations invalidateEnv.tsfixes [FEATURE] Add
GET /api/streams/export.csvendpoint #601