[SSF-232] Edit & Delete Donation Items Frontend#199
Conversation
dburkhart07
left a comment
dburkhart07
left a comment
There was a problem hiding this comment.
looking good so far! just some small things
|
Just got an update that admins should also be able to edit/delete donation items - this PR isn't completely ready atm |
dburkhart07
left a comment
dburkhart07
left a comment
There was a problem hiding this comment.
all small things, pr looks really good!!
Yurika-Kan
left a comment
Yurika-Kan
left a comment
There was a problem hiding this comment.
found some regression bugs! --> addressing effects race condition, deep linking
lmk if you need more clarification~
| onDelete: () => void; | ||
| } | ||
|
|
||
| interface DonationRow { |
There was a problem hiding this comment.
this reimplements newDonationFormModal's item editing fully with duplicate DonationRow, addRow/deleteRow/handleChange, donation items table. can we extract the item-rows component instead of duplicating it here? that way we use the same logic for validating donation items (regardless of if it was created or edited/updated)
| .find((d) => d.donation.donationId === id); | ||
| if (match) { | ||
| setSelectedViewDetailsDonation(match.donation); | ||
| } else navigate(ROUTES.FM_DONATION_MANAGEMENT); |
There was a problem hiding this comment.
bug: FM dashboard --> donation details via "View Donation Details/Requirements" is broken
when this page is initially rendering
this useEffect "B" runs on mount alongside the init effect "A" (L139). however that init is async & yields at its first await:
const init = async () => {
try {
const fmId = await ApiClient.getCurrentUserFoodManufacturerId();
so effect "B" runs before effect "A" can get to fetchDonations (L144) which set donation statuses (L91). statusDonations const is still exactly what useState initialized it to (L48-54, empty buckets). in effect "B", match (L165) is undefined, leaving us to the navigate (L170) case where it strips the ?donationId= param before data loads = modal never opens!
fix: gate effect "B" on loading complete so the match waits for populated donation data. add
if (loading) return;
to effect "B" at the top & also gate on loading so that it navigates when genuinely
| <DonationDetailsModal | ||
| donation={selectedViewDetailsDonation} | ||
| isOpen={selectedViewDetailsDonation !== null} | ||
| onClose={() => setSelectedViewDetailsDonation(null)} |
There was a problem hiding this comment.
bug: onclose does not clear the ?donationId= param in the URL, resulting in possible modal reopening after closing due to the param still being there.
fix: handle this how adminDonation.tsx already does by clearing the param on close
setSelectedViewDetailsDonation(null);
navigate(ROUTES.FM_DONATION_MANAGEMENT, { replace: true });
}}``` (adminDonation.tsx:313-315)
| }} | ||
| onSuccess={() => { | ||
| setAlertMessage( | ||
| 'Successfully deleted donation items.', |
There was a problem hiding this comment.
Successfully deleted donation items --> Successfully deleted donation
since delete just removed the whole thing
| }} | ||
| onSuccess={() => { | ||
| setAlertMessage( | ||
| 'Successfully deleted donation items.', |
3 participants
ℹ️ Issue
Closes SSF-232
📝 Description
For
/fm-donation-management:✔️ Verification
Tested cases for:
editDonationItemsbackend)Edit/Delete Buttons:
Editing View:
Delete Modal:
🏕️ (Optional) Future Work / Notes
what's the difference between "matched" and "available" donations?
i have an "available" donation with allocations, so no edit/delete buttons are displayed (screenshot).
if some "available" donations have edit/delete buttons (no allocations) and some don't have these buttons (because they have allocations), i think it's kind of confusing for the user since both donations are under "available." it would be helpful to put a note/visual indicator that "available" donations with allocations cannot be edited/deleted.