Skip to content

Conversation

@adamrefaey
Copy link
Collaborator

@adamrefaey adamrefaey commented Apr 24, 2025

Change

This pull request introduces several enhancements and fixes across the backend and frontend codebases, focusing on improving error handling, user experience, and code maintainability. The key changes include refining backend logic for report processing, enhancing frontend API handling and UI components, and improving test coverage and mocking strategies.

Backend Enhancements:

  • Improved Error Handling in ReportsController: Added specific checks for IN_PROGRESS and UNPROCESSED statuses to provide more descriptive error messages when fetching reports.
  • Refined Query Logic in ReportsService: Updated methods to use onlyProcessed instead of withFailed, ensuring that only processed reports are retrieved by default. This change affects both findAll and findLatest methods. [1] [2] [3]

Frontend Enhancements:

  • Improved API Handling:

    • Updated markReportAsRead and toggleReportBookmark to include better error messages and authentication configuration. [1] [2]
    • Enhanced the ProcessingPage to handle successful processing responses and redirect users to the report detail page.
  • UI/UX Improvements:

    • Refined the styling of report detail items in ReportDetailPage.scss, including updated colors, padding, and typography for better readability and aesthetics.
    • Enhanced the LabValueItem component to intelligently parse suggestions into bullet points and improve status label handling. [1] [2] [3]

Testing and Mocking:

  • Expanded Test Mocking: Added mocks for @aws-amplify/auth and getAuthConfig in reportService.test.ts to handle authentication and avoid dependencies during testing. [1] [2] [3]

Additional Improvements:

  • Enhanced Data Fetching in Reports Pages:

    • Enabled automatic refetching of reports when navigating to the ReportsListPage to ensure data freshness. [1] [2] [3]
    • Added options to always refetch and refresh data in useGetLatestReports.
  • Simplified Upload Modal Behavior: Removed the automatic opening of the upload modal on the UploadPage and provided a manual trigger for better user control. [1] [2] [3]

Does this PR introduce a breaking change?

{...}

What needs to be documented once your changes are merged?

{...}

Additional Comments

{...}

- Update markReportAsRead to include auth config in the request.
- Modify useGetLatestReports to always refetch data on mount and window focus.
- Improve ProcessingPage to handle response and redirect upon processing completion.
- Add location tracking in ReportsListPage to refetch reports on navigation.
@adamrefaey adamrefaey self-assigned this Apr 24, 2025
@adamrefaey adamrefaey changed the title Enhance report handling and data fetching: [ADE-66] fixes and improvements to uploading, processing, and viewing a report Apr 24, 2025
@adamrefaey adamrefaey requested review from GuidoBR and Copilot April 24, 2025 22:39
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This pull request refines both backend and frontend logic to improve report handling, error messaging, and user experience. Key changes include switching to manual upload modal triggering and enhancing report filtering with clearer status conditions, along with updated API error messages and test mocks.

  • Removed automatic upload modal opening on the Upload page and added a manual trigger.
  • Improved suggestions parsing and dynamic status label handling in LabValueItem.
  • Updated report fetching and processing logic with refined error handling and query filters.
  • Expanded test mocks to better handle authentication dependencies.

Reviewed Changes

Copilot reviewed 10 out of 11 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
frontend/src/pages/Upload/UploadPage.tsx Changed modal state from auto-open to manually triggered, enhancing user control.
frontend/src/pages/Reports/components/LabValueItem.tsx Added a robust suggestions parser and improved status label handling based on item status.
frontend/src/pages/Reports/ReportsListPage.tsx Added refetch logic based on location changes to ensure fresh report data.
frontend/src/pages/Processing/ProcessingPage.tsx Enhanced response handling by checking processing status before routing.
frontend/src/common/hooks/useReports.ts Updated report fetching to always refetch on mount and window focus for freshness.
frontend/src/common/components/Upload/UploadModal.tsx Updated success-state handling with an early return to avoid duplicate state resets.
frontend/src/common/api/reportService.ts Refined API calls with improved error message fallbacks and updated auth config usage.
frontend/src/common/api/tests/reportService.test.ts Added enhanced mocks for authentication and API functions to mitigate external dependencies.
backend/src/reports/reports.service.ts Changed query logic to filter for only processed reports, aligning with updated business rules.
backend/src/reports/reports.controller.ts Added additional error responses for reports in progress or pending processing.
Files not reviewed (1)
  • frontend/src/pages/Reports/ReportDetailPage.scss: Language not supported
Comments suppressed due to low confidence (3)

frontend/src/pages/Reports/ReportsListPage.tsx:69

  • [nitpick] Consider potential performance implications of refetching reports on every mount; ensure tests cover scenarios with frequent navigation to avoid redundant network requests.
refetchOnMount: true,

backend/src/reports/reports.service.ts:65

  • Verify that switching from filtering out failed statuses to explicitly including only processed reports fully meets the intended business requirements.
const processingStatusFilter = 'processingStatus = :processedStatus';

frontend/src/common/components/Upload/UploadModal.tsx:119

  • The early return after a successful upload prevents further state cleanup; please verify that all necessary resets have been executed before returning.
return;

@GuidoBR GuidoBR merged commit aca7eab into main Apr 24, 2025
@adamrefaey adamrefaey deleted the ADE-66 branch April 24, 2025 23:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants