Skip to content

Conversation

@adamrefaey
Copy link
Collaborator

@adamrefaey adamrefaey commented Apr 23, 2025

Change

This pull request introduces several enhancements and fixes across the backend and frontend of the application. Key updates include improved error handling in the reports controller, enhanced filtering logic in the reports service, the addition of a new method to delete reports, and minor changes in the frontend and other services for better usability and consistency.

Backend Enhancements:

Reports Controller:

  • Added NotFoundException to handle cases where a report is not found or its processing status is FAILED in the getReport method. [1] [2]

Reports Service:

  • Enhanced filtering in the findAll and findLatest methods to optionally exclude reports with a FAILED processing status using a new withFailed parameter. [1] [2] [3] [4]
  • Introduced a new deleteReport method to allow users to delete reports by ID, with proper validation and error handling for permissions and missing resources.

Other Backend Changes:

  • Updated the PerplexityService to refine the user prompt for better clarity and conciseness.

Frontend Update:

  • Fixed a namespace issue in the ReportHeader component to ensure the correct translation key is used for report categories.

Does this PR introduce a breaking change?

{...}

What needs to be documented once your changes are merged?

{...}

Additional Comments

{...}

@adamrefaey adamrefaey self-assigned this Apr 23, 2025
@adamrefaey adamrefaey requested review from GuidoBR and Copilot April 23, 2025 16:59
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 PR enhances report handling by adding stricter validation for report retrieval, filtering options for queries, and a new report deletion feature while updating the UI translation and user prompt messaging.

  • Enforces error handling in report retrieval if a report is not found or if processing has failed.
  • Adds filtering logic to exclude reports with a FAILED processing status in both listing and latest queries.
  • Introduces a deleteReport method for secure report deletion with proper exception management.

Reviewed Changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.

File Description
frontend/src/pages/Reports/components/ReportHeader.tsx Updated translation namespace for improved localization.
backend/src/services/perplexity.service.ts Refined the user prompt for clarity and concise explanation.
backend/src/reports/reports.service.ts Enhanced query filtering logic and added deleteReport functionality.
backend/src/reports/reports.controller.ts Modified getReport method to validate report existence and status.

@adamrefaey adamrefaey requested a review from Copilot April 23, 2025 17:12
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 PR enhances report handling and error management across both backend and frontend components. Key changes include stricter report validation in the controller, improved filtering in report retrieval methods, a new report deletion feature, and updates to UI localization and user prompts.

Reviewed Changes

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

File Description
frontend/src/pages/Reports/components/ReportHeader.tsx Updated the translation namespace for report category localization
backend/src/services/perplexity.service.ts Adjusted the user prompt for medical text explanation to clarify required formatting
backend/src/reports/reports.service.ts Added filtering options to retrieval methods and introduced a new report deletion feature
backend/src/reports/reports.controller.ts Enhanced report validation in the getReport method to enforce stricter error checking

@adamrefaey adamrefaey requested a review from Copilot April 23, 2025 17:15
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

Enhance report retrieval and deletion logic while improving error handling and consistency across services and the frontend.

  • Update translation namespace in the ReportHeader component for correct report category mapping.
  • Enhance filtering in report queries via an optional parameter and add a new deleteReport method with proper error handling.
  • Refactor error handling in the PerplexityService to tighten the user prompt and adjust output formatting.

Reviewed Changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.

File Description
frontend/src/pages/Reports/components/ReportHeader.tsx Updated translation key namespace to use 'reportDetail'.
backend/src/services/perplexity.service.ts Adjusted the user prompt for improved clarity and concise output.
backend/src/reports/reports.service.ts Enhanced filtering in report queries and added a deleteReport method.
backend/src/reports/reports.controller.ts Updated getReport to throw NotFoundException when report is missing or failed.
Comments suppressed due to low confidence (2)

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

  • [nitpick] Consider renaming the 'withFailed' parameter to 'includeFailed' to more clearly indicate that it controls whether failed reports are included in the results.
async findAll(userId: string, withFailed = false): Promise<Report[]> {

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

  • [nitpick] Consider renaming the 'withFailed' parameter to 'includeFailed' for clearer intent in controlling the inclusion of failed reports.
async findLatest(queryDto: GetReportsQueryDto, userId: string, withFailed = false): Promise<Report[]> {

@adamrefaey adamrefaey merged commit d335bb9 into main Apr 23, 2025
2 checks passed
@adamrefaey adamrefaey deleted the ADE-66 branch April 23, 2025 17:18
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