Yk/fm bugbash - #213
Open
Yurika-Kan wants to merge 12 commits into
Open
Conversation
useAlert overwrote its single error slot on each call, so concurrent fetch failures (stats, reminders, donations) silently dropped all but the last. Now same-status errors append as separate lines, with exact duplicates deduped. Affects all four dashboards (admin/FM/pantry/ volunteer) since they share this hook.
fetchDonations() reset all three status tables' current page to 1 on every successful mutation, even ones scoped to a different table. Only reset pagination on the initial load/deep-link fetch.
Delete button had no disabled/loading state, so a fast double-click fired two delete requests; the second 404s after the first succeeds and shows a false "could not be deleted" error.
Dialog.Root mounted via `{x && <Modal isOpen={true} />}` opens already
`open=true` on its first render, so focus-trap activates before the
CloseButton ref settles and finds zero focusable elements, throwing
"Your focus-trap needs to have at least one focusable element" to the
error boundary. Add lazyMount/unmountOnExit to every Dialog.Root so Ark
UI defers mounting the content tree until the open transition begins.
There was a problem hiding this comment.
🟡 Changes recommended
Four moderate review findings remain unresolved.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
Improves dashboard resilience with retryable error states, alert aggregation, multi-manufacturer support, and donation pagination updates.
Changes:
- Adds dashboard fetch failure states and retry actions.
- Supports multiple food manufacturers.
- Updates alert handling and donation refresh behavior.
File summaries
| File | Summary | Review findings |
|---|---|---|
apps/frontend/src/hooks/alert.ts |
Aggregates duplicate-status alerts. | Moderate (3 votes): repeated errors after timeout are not shown again; issue a new alert ID or reset state. |
apps/frontend/src/containers/volunteerDashboard.tsx |
Adds section-level retry handling. | No final findings. |
apps/frontend/src/containers/pantryDashboard.tsx |
Adds section-level retry handling. | No final findings. |
apps/frontend/src/containers/foodManufacturerDashboard.tsx |
Supports multiple manufacturers and donation retries. | Moderate (3 votes): track reminders failures separately and render an error/retry state instead of an empty result. |
apps/frontend/src/containers/adminDashboard.tsx |
Adds section-level retry handling. | No final findings. |
apps/frontend/src/containers/foodManufacturerDonationManagement.tsx |
Updates pagination refresh and missing-donation handling. | Moderate (3 votes): clamp preserved pages to the new last page. Moderate (1 vote): only redirect for missing donations after a successful fetch. |
Review details
Suppressed comments (1)
apps/frontend/src/containers/foodManufacturerDonationManagement.tsx:168
- If
fetchDonations()fails, it catches the request error and leavesstatusDonationsempty, whileinitstill clearsloading. This effect then reports “Donation not found” and redirects for a valid donation whenever the API is unavailable, masking the fetch error and preventing recovery. Only perform this not-found redirect after confirming that the donations request completed successfully.
} else {
setAlertMessage('Donation not found.', AlertStatus.ERROR);
navigate(ROUTES.FM_DONATION_MANAGEMENT, { replace: true });
- Files reviewed: 29/29 changed files
- Comments generated: 3
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
|
|
||
| // Fetch all donations on component mount and sorts them into their appropriate status lists | ||
| const fetchDonations = async () => { | ||
| const fetchDonations = async (resetPages = false) => { |
| if (prev && prev.status === status) { | ||
| const lines = prev.message.split('\n'); | ||
| if (lines.includes(message)) { | ||
| return prev; |
Covers the fix requiring ozPerItem/estimatedValue >= 0.01 before Complete Required Actions submit is enabled. Adds jsdom test environment to vite config and @testing-library/dom dependency. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…y view Upcoming Donations card sent FM reps to a read-only donation-details view, while the equivalent reminder email linked straight to the Resubmit modal. Point the card at ?resubmitDonationId= to match, and relabel the card and link text accordingly. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
This reverts commit f042033.
Yurika-Kan
commented
Sep 13, 2026
| reminder.donation.foodManufacturer?.foodManufacturerName | ||
| } | ||
| linkText="View Donation Requirements" | ||
| linkText="Submit Donation" |
Collaborator
Author
There was a problem hiding this comment.
"submit recurring donation"?
i think it makes more sense to navigate to the resubmit functionality here but i dont want the wording to be misleading.
needs to be clear that they still need to manually submit it, this is for email reminders
Deleting/resubmitting the last item on a status's last page left currentPages stale, so PaginationControl hid itself (totalPages<=1) while the stale page rendered no rows.
…y-state copy Admin/FM/pantry/volunteer dashboards silently showed no stats block on getUserStats failure with no way to recover without reload. Adds a retry button gated on a per-dashboard statsFetchFailed flag.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
ℹ️ Issue
FM Bug bash
bugbasheet
📝 Description
✔️ Verification
🏕️ (Optional) Future Work / Notes
Did you notice anything ugly during the course of this ticket? Any bugs, design challenges, or unexpected behavior? Write it down so we can clean it up in a future ticket!