[ADE-67] Refactor action button handlers to manage processing state during report deletion and new uploads #131
Conversation
…ort deletion and new uploads
There was a problem hiding this comment.
Pull Request Overview
This PR refactors the action button handlers to better manage the processing state during report deletion and new uploads. The changes update the signatures of onDiscard and onNewUpload to accept a state callback, remove the local isDeleting state in favor of a unified isProcessing flag, and delegate axios deletion logic to the ReportDetailPage handlers.
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| frontend/src/pages/Reports/components/ActionButtons.tsx | Refactored props and internal state to centralize processing state handling. |
| frontend/src/pages/Reports/ReportDetailPage.tsx | Updated action handler functions to incorporate processing state updates with axios deletion logic. |
There was a problem hiding this comment.
Pull Request Overview
This PR refactors the action button handlers for report deletion and new uploads to manage a unified processing state.
- Updated the ActionButtons component to use a single processing indicator instead of separate deletion state.
- Modified the handler functions in ReportDetailPage to propagate and manage processing state through async operations.
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| frontend/src/pages/Reports/components/ActionButtons.tsx | Removed redundant imports and state; updated prop types to use processing state; simplified the async handlers. |
| frontend/src/pages/Reports/ReportDetailPage.tsx | Updated the handler functions to set processing state appropriately; removed the conditional branch for report deletion when uploading a new report. |
There was a problem hiding this comment.
Pull Request Overview
This PR refactors the action button handlers to better manage the processing state during report deletion and new uploads. Key changes include updating function signatures for onDiscard and onNewUpload to accept a state-setting callback, consolidating the processing state in place of a separate deletion flag, and modifying the handleNewUpload behavior in the report detail page.
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| frontend/src/pages/Reports/components/ActionButtons.tsx | Removed redundant deletion state and updated handler signatures to use a unified processing state |
| frontend/src/pages/Reports/ReportDetailPage.tsx | Updated handleDiscard and handleNewUpload to use the processing state; handleNewUpload now always attempts to delete a report before navigating to upload |
Comments suppressed due to low confidence (1)
frontend/src/pages/Reports/ReportDetailPage.tsx:121
- [nitpick] The function name 'handleNewUpload' might be misleading since it performs a deletion before navigating to the new upload page. Consider renaming it to better reflect its dual responsibility.
const handleNewUpload = async (setIsProcessing: (isProcessing: boolean) => void) => {
|
|
||
| const handleNewUpload = () => { | ||
| history.push('/tabs/upload'); | ||
| const handleNewUpload = async (setIsProcessing: (isProcessing: boolean) => void) => { |
There was a problem hiding this comment.
what does this method do?
|
|
||
| // Handle action buttons | ||
| const handleDiscard = async () => { | ||
| const handleDiscard = async (setIsProcessing: (isProcessing: boolean) => void) => { |
There was a problem hiding this comment.
✅ Change applied: Added a finally block to handleDiscard to ensure isProcessing is always reset, even if the API call fails. This improves reliability and user experience. Thank you for the suggestion!
|
|
||
| const handleNewUpload = () => { | ||
| history.push('/tabs/upload'); | ||
| const handleNewUpload = async (setIsProcessing: (isProcessing: boolean) => void) => { |
There was a problem hiding this comment.
✅ Change applied: Added a check for reportId in handleNewUpload to prevent unintended API calls and provide user feedback if reportId is missing. This makes the code more robust. Thanks for the suggestion!
|
|
||
| const handleNewUpload = () => { | ||
| history.push('/tabs/upload'); | ||
| const handleNewUpload = async (setIsProcessing: (isProcessing: boolean) => void) => { |
There was a problem hiding this comment.
✅ Change applied: Now, handleNewUpload checks if reportId is defined before attempting deletion, and provides a toast error if not. This prevents unintended API calls and improves user feedback. Thanks for the suggestion!
|
|
||
| const handleNewUpload = () => { | ||
| history.push('/tabs/upload'); | ||
| const handleNewUpload = async (setIsProcessing: (isProcessing: boolean) => void) => { |
There was a problem hiding this comment.
✅ Change applied: handleNewUpload now checks for reportId before deleting and shows an error if it's missing. This avoids unintended behavior. Thanks for the suggestion!
|
|
||
| const handleNewUpload = () => { | ||
| history.push('/tabs/upload'); | ||
| const handleNewUpload = async (setIsProcessing: (isProcessing: boolean) => void) => { |
There was a problem hiding this comment.
This method (handleNewUpload) deletes the current report (if a valid reportId exists) and then opens the upload modal so the user can upload a new report. This ensures that only one report is active at a time, and helps keep the user's report archive clean.
Change
Refactor action button handlers to manage processing state during report deletion and new uploads
Does this PR introduce a breaking change?
{...}
What needs to be documented once your changes are merged?
{...}
Additional Comments
{...}