Conversation
Greptile SummaryThis PR adds a CSV export feature for volunteer availability data in the Member Management page. An "Export CSV" button is added to the Volunteers tab (admin-only, gated by the existing
Confidence Score: 4/5
|
| Filename | Overview |
|---|---|
| src/components/members/pages/view-volunteers-view.tsx | Adds Export CSV button with disabled-query pattern, CSV builder, escape helper, and download trigger. Clean implementation; availability output contains ` |
| src/utils/availabilityUtils.ts | Adds formatAvailabilityByDay to convert availability bitstring into human-readable daily ranges. Edge cases (empty, invalid, end-of-day boundary) are handled correctly. |
| src/server/services/entity/volunteerService.ts | Adds getVolunteersForExport with optional search filtering. No pagination limit, which is intentional for export but could be a concern with large datasets. |
| src/server/api/routers/volunteer-router.ts | Adds exportAvailability query endpoint with users: ["view"] permission (admin-only). Consistent with existing authorization patterns. |
| src/models/api/volunteer.ts | Adds VolunteerExportInput Zod schema with optional search field. Clean and minimal. |
| src/test/unit/availability-utils.test.ts | New unit tests for formatAvailabilityByDay covering invalid input, single range, and multi-range/multi-day scenarios. Test assertions are correct, but one inline comment is misleading. |
Last reviewed commit: 60335f5
| // Wednesday 18:00-20:00 (slots offset by 2 days) | ||
| const mondaySlots = [0, 1, 28, 29, 30]; |
There was a problem hiding this comment.
Misleading test comment
The comment says "Wednesday 18:00-20:00" but only 2 slots are set (wednesdayOffset + 36 and wednesdayOffset + 37), which is 1 hour, not 2. The assertion on line 47 correctly expects "Wed 18:00-19:00". The comment should match:
| // Wednesday 18:00-20:00 (slots offset by 2 days) | |
| const mondaySlots = [0, 1, 28, 29, 30]; | |
| // Wednesday 18:00-19:00 (slots offset by 2 days) | |
| const wednesdaySlots = [wednesdayOffset + 36, wednesdayOffset + 37]; |
| let builder = this.db | ||
| .select(getViewColumns(volunteerUserView)) | ||
| .from(volunteerUserView) | ||
| .$dynamic(); | ||
|
|
||
| if (hasQuery) { | ||
| builder = builder | ||
| .where( | ||
| buildSearchCondition( | ||
| [ | ||
| volunteerUserView.name, | ||
| volunteerUserView.lastName, | ||
| volunteerUserView.email, | ||
| ], | ||
| queryInput, | ||
| ), | ||
| ) | ||
| .orderBy(desc(similarity!)); | ||
| } else { | ||
| builder = builder.orderBy( | ||
| volunteerUserView.lastName, | ||
| volunteerUserView.name, | ||
| ); | ||
| } | ||
|
|
||
| const rows = await builder.execute(); |
There was a problem hiding this comment.
Unbounded query may return very large result set
Unlike getVolunteersForRequest, this method has no .limit() on the query. If the volunteer table grows large, this will fetch every row in a single query, which could cause high memory usage on both the database and app server, and a slow response for the client.
Consider adding a reasonable upper bound (e.g., .limit(5000)) as a safety net, or paginating the export on the frontend side.
Summary