Conversation
…esponse, and partner
✅ Deploy Preview for hope-for-haiti ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
Greptile SummaryImplements advanced table filtering across multiple admin pages (Account Management, Distributions, Shipments, Sign-offs, Donor Offers) by adding Key changes:
Critical issues found:
Confidence Score: 2/5
Important Files Changed
|
src/services/donorOfferService.ts
Outdated
| const [donorOffers] = await Promise.all([ | ||
| db.donorOffer.findMany(query), | ||
| db.donorOffer.count({ where }), | ||
| ]); |
There was a problem hiding this comment.
The total variable from db.donorOffer.count() is computed but never used (was removed on line 447). Later on line 482, the function returns total: filteredOffers.length, which breaks pagination because the total count doesn't account for the database-level filters - it only counts the post-filtering results. This causes incorrect pagination metadata.
| const [donorOffers] = await Promise.all([ | |
| db.donorOffer.findMany(query), | |
| db.donorOffer.count({ where }), | |
| ]); | |
| const [donorOffers, total] = await Promise.all([ | |
| db.donorOffer.findMany(query), | |
| db.donorOffer.count({ where }), | |
| ]); |
There was a problem hiding this comment.
You need to restore the total variable in the destructuring on line 447 and use it in the return statement instead of filteredOffers.length.
| const [donorOffers] = await Promise.all([ | |
| db.donorOffer.findMany(query), | |
| db.donorOffer.count({ where }), | |
| ]); | |
| const [donorOffers, total] = await Promise.all([ | |
| db.donorOffer.findMany(query), | |
| db.donorOffer.count({ where }), | |
| ]); |
Then in your return statement (around line 482), make sure you're returning the database total instead of filteredOffers.length:
return { donorOffers: filteredOffers, total };The current implementation breaks pagination because filteredOffers.length only reflects items after client-side filtering, not the total count from the database query. Pagination metadata needs the database count to work correctly.
There was a problem hiding this comment.
Looking at the current implementation, I can see the issue. The code computes total from db.donorOffer.count() on line 450 but then discards it on line 447. Later, the function returns filteredOffers.length instead of total, which breaks pagination.
However, there's a complication: you're doing post-query filtering for the "All Partners Responded" field. This means the total from the database won't match filteredOffers.length after client-side filtering.
You need to choose one approach:
Option 1 (Recommended): Move all filtering to the database level
- Add
allPartnersRespondedas a computed field in Prisma or pre-calculate it - Remove client-side filtering
- Use database
totalfor pagination
Option 2: Keep client-side filtering but fix the count
- You'll need to run the count query WITH the same post-filtering logic
- This means fetching all records matching the base query, filtering them, then counting
Right now you're in a broken middle state where:
- Database returns N records with total = N
- Client filters down to M records (M < N)
- You return
total: filteredOffers.length(M) but pagination still uses the original query
The fix depends on your filtering requirements. If you must keep client-side filtering for "All Partners Responded", you'll need to fetch all matching records to get an accurate count, which defeats the purpose of pagination.
Quick fix for now:
| const [donorOffers] = await Promise.all([ | |
| db.donorOffer.findMany(query), | |
| db.donorOffer.count({ where }), | |
| ]); | |
| const [donorOffers, total] = await Promise.all([ |
And ensure line 482 returns:
return { donorOffers: filteredOffers, total };But understand this will show incorrect pagination when "All Partners Responded" filter is active.
There was a problem hiding this comment.
should be fixed now in the fix two threads below this
| if (filters?.responseDeadline && filters.responseDeadline.type === "date") { | ||
| const dateCondition: Record<string, string> = {}; | ||
| if (filters.responseDeadline.gte) dateCondition.gte = filters.responseDeadline.gte; | ||
| if (filters.responseDeadline.lte) dateCondition.lte = filters.responseDeadline.lte; | ||
| (where as Record<string, unknown>).partnerResponseDeadline = dateCondition; | ||
| } |
There was a problem hiding this comment.
Filter is mapped to wrong field - responseDeadline filter maps to partnerResponseDeadline, but it should map to donorResponseDeadline based on the frontend column id at AdminDonorOffersScreen.tsx:87 which is "donorResponseDeadline".
| if (filters?.responseDeadline && filters.responseDeadline.type === "date") { | |
| const dateCondition: Record<string, string> = {}; | |
| if (filters.responseDeadline.gte) dateCondition.gte = filters.responseDeadline.gte; | |
| if (filters.responseDeadline.lte) dateCondition.lte = filters.responseDeadline.lte; | |
| (where as Record<string, unknown>).partnerResponseDeadline = dateCondition; | |
| } | |
| if (filters?.donorResponseDeadline && filters.donorResponseDeadline.type === "date") { | |
| const dateCondition: Record<string, string> = {}; | |
| if (filters.donorResponseDeadline.gte) dateCondition.gte = filters.donorResponseDeadline.gte; | |
| if (filters.donorResponseDeadline.lte) dateCondition.lte = filters.donorResponseDeadline.lte; | |
| (where as Record<string, unknown>).donorResponseDeadline = dateCondition; | |
| } |
There was a problem hiding this comment.
Pretty confident the initial implementation is correct. There are two deadline columns within AdminDonorOffersScreen. There is responseDeadline on line 116 and donorResponseDeadline on line 131.
src/services/donorOfferService.ts
Outdated
| let filteredOffers = mappedOffers; | ||
| if (filters?.allPartnersResponded && filters.allPartnersResponded.type === "enum") { | ||
| const wantAllPartnersResponded = filters.allPartnersResponded.values.includes("Yes"); | ||
| filteredOffers = mappedOffers.filter((offer) => { | ||
| const allPartnersResponded = offer.invitedPartners.length > 0 && | ||
| offer.invitedPartners.every((p) => p.responded); | ||
|
|
||
| return wantAllPartnersResponded ? allPartnersResponded : !allPartnersResponded | ||
| }); | ||
| } |
There was a problem hiding this comment.
Post-query filtering after pagination breaks expected behavior - filtering happens AFTER the database query (which already applied pagination), so users see incomplete results. For example, with 100 total offers where 20 match the filter, if pageSize=20, only the first 20 offers are queried, then filtered down to maybe 2-3 results, leaving 17+ empty slots on the page.
This needs to be either: 1) moved before the database query as a WHERE clause, or 2) the total count needs to reflect post-filter results and pagination recalculated.
There was a problem hiding this comment.
The allPartnersResponded filter is already correctly implemented at the database level (lines 426-451). The post-query filtering in lines 471-480 is redundant and should be removed:
| let filteredOffers = mappedOffers; | |
| if (filters?.allPartnersResponded && filters.allPartnersResponded.type === "enum") { | |
| const wantAllPartnersResponded = filters.allPartnersResponded.values.includes("Yes"); | |
| filteredOffers = mappedOffers.filter((offer) => { | |
| const allPartnersResponded = offer.invitedPartners.length > 0 && | |
| offer.invitedPartners.every((p) => p.responded); | |
| return wantAllPartnersResponded ? allPartnersResponded : !allPartnersResponded | |
| }); | |
| } | |
| return {donorOffers: mappedOffers, total} |
The database query already filters offers based on whether all partners responded, so applying the same filter again after pagination breaks the expected behavior. Remove lines 471-480 and just return the mapped offers directly.
There was a problem hiding this comment.
The redundant post-query filtering has been removed. The code now correctly returns results directly from the database query without refiltering:
return {donorOffers: mappedOffers, total}The allPartnersResponded filter is properly implemented at the database level using the raw SQL query (lines 426-451), and there's no duplicate filtering after pagination. The issue is resolved.
| if (filters?.value && filters?.value.type === "enum") { | ||
| const displayNames = filters.value.values; | ||
| const enumValueArr = Object.entries(shippingStatusToText); | ||
| const enumValues = displayNames | ||
| .map((name) => enumValueArr.find(([,text]) => text === name)?.[0]) | ||
| .filter(Boolean); | ||
| (where as Record<string, unknown>).value = { | ||
| in: enumValues | ||
| }; | ||
| } |
There was a problem hiding this comment.
Filters applied with where clause only used when isCompleted is undefined (line 143-150), but ignored when isCompleted is defined (line 70-141). This means status filters won't work on the shipments/signoffs tables that use the isCompleted parameter.
There was a problem hiding this comment.
The issue I raised is valid - the status filter mapping you added (lines 52-61) only applies when isCompleted is undefined. When isCompleted is true or false (which is the case for shipments and sign-offs tables), the code takes a completely different path using raw SQL queries (lines 70-141), bypassing the where clause entirely.
This means status filtering won't work on the shipments/sign-offs tables that pass the isCompleted parameter. You need to either:
- Apply the display name → enum value mapping before calling
getShipments(), or - Incorporate filter logic into the raw SQL query path (more complex)
The simpler fix is option 1 - do the mapping in ShipmentsTable.tsx and SignOffsTable.tsx before passing filters to the service.
There was a problem hiding this comment.
You've correctly fixed the issue! The status filter mapping now works on the raw SQL query path (lines 70-90 in shippingStatusService.ts:52-61), so filtering will work for both shipments and sign-offs tables. The implementation properly converts display names to enum values before injecting them into the SQL query. Good work!
|
Revision was simple but effective, great work |




Description
Resolves ticket number: #280
Explain what your code changes:
Issue 1: Account Management
Relevant Files: AccountManagementPage/page.tsx
The Role filter was not working properly, so added a filterType: "enum" was added along with filterOptions being the prisma UserTypes
Issue 2: Distributions
Relevant Files: DistributionTable.tsx and distributionService.tsx
Before
After
Issue 3: Shipments & Signoffs Tables
Relevant Files: ShipmentsTable.tsx, SignOffsTable.tsx, shippingStatusService.ts
Issue 4: Donor Offers Page(Unfinalized & Finalized)
Relevant Files: AdminDonorOffersScreen.tsx, donorOffer.types.ts, donorOfferService.ts
Before
After
List the steps you took to test your code: I used seed.ts, added mock data manually using the Prisma DB, and added the csv data to the donor offers to filter.
Checklist